-
Notifications
You must be signed in to change notification settings - Fork 3.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
GH-35404: [Docs] Fix logo url and temporarily pin sphinx to 5.x to #35405
GH-35404: [Docs] Fix logo url and temporarily pin sphinx to 5.x to #35405
Conversation
|
@github-actions crossbow submit preview-docs |
Revision: 7f2d7b4 Submitted crossbow builds: ursacomputing/crossbow @ actions-f17fd805e0
|
@github-actions crossbow submit preview-docs |
Revision: 750b3cb Submitted crossbow builds: ursacomputing/crossbow @ actions-0a103adb20
|
This seems to have worked: http://crossbow.voltrondata.com/pr_docs/35405/ |
@@ -1,6 +1,6 @@ | |||
|
|||
<a class="navbar-brand" href="{{ pathto(master_doc) }}"> | |||
<img src="{{ pathto('_static/' + logo_url, 1) }}" class="logo" alt="logo"> | |||
<img src="{{ logo_url|e }}" class="logo" alt="logo"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this work only with Sphinx 5?
If this works with both of Sphinx 5 and 6, how about removing ,<6
for sphinx
from this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This works for both but there are other issue (#35403) that require us to pin to <6 for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, @assignUser is spot on. This fixes the logo issue (#35406) regardless of the sphinx version, but we still need to fix the jquery dependency issue for being able to use sphinx 6
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. I see.
Ah, thanks for catching this. I thought I tested the logo_url fix but maybe I didn't test with the same sphinx version 😦 |
Benchmark runs are scheduled for baseline = b9cc5df and contender = 57c956e. 57c956e is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
['Python', 'R'] benchmarks have high level of regressions. |
… to (apache#35405) See apache#35403 about requiring some fixes on our side to support sphinx 6. Until then, let's pin sphinx to <6. * Closes: apache#35404 * Closes: apache#35406 Authored-by: Joris Van den Bossche <jorisvandenbossche@gmail.com> Signed-off-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>
… to (apache#35405) See apache#35403 about requiring some fixes on our side to support sphinx 6. Until then, let's pin sphinx to <6. * Closes: apache#35404 * Closes: apache#35406 Authored-by: Joris Van den Bossche <jorisvandenbossche@gmail.com> Signed-off-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>
… to (apache#35405) See apache#35403 about requiring some fixes on our side to support sphinx 6. Until then, let's pin sphinx to <6. * Closes: apache#35404 * Closes: apache#35406 Authored-by: Joris Van den Bossche <jorisvandenbossche@gmail.com> Signed-off-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>
See #35403 about requiring some fixes on our side to support sphinx 6. Until then, let's pin sphinx to <6.